Skip to content

Conversation

lannuttia
Copy link
Contributor

@lannuttia lannuttia commented Aug 5, 2025

Description

When an APIRoute subclass would overwrite the matches method with an implementation that depended on non-standard fields existing on the HTTP connection scope, this would cause a failure when the OpenTelemetryMiddleware tried to get the default span details for the incoming request. This has been fixed by using the matches implementation on the Route class for any subclass of Route. This should be sufficient since the only information we are trying to get from that method is the path for the request.

Fixes #3671

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

This has been validated with automated tests that are included in this pull request.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Copy link

linux-foundation-easycla bot commented Aug 5, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@lannuttia lannuttia marked this pull request as ready for review August 5, 2025 22:55
@lannuttia lannuttia changed the title Fix FastAPI issue with APIRouter subclasses Fix FastAPI issue with APIRoute subclasses Aug 5, 2025
Fixes: open-telemetry#3671

When an APIRoute subclass would overwrite the matches method with an implementation that depended
on non-standard fields existing on the HTTP connection scope, this would cause a failure when the
OpenTelemetryMiddleware tried to get the default span details for the incoming request. This has
been fixed by using the matches implementation on the Route class for any subclass of Route. This
should be sufficient since the only information we are trying to get from that method is the path
for the request.
This commit adds tests that illustrate the original issue that was being
experienced for custom api route implementations when they depended on
non-standard fields existing on the ASGI HTTP connection scope. Before
the fix was implemented, the inclusion of a custom API route in the
FastAPI application would cause an exception to be raised inside the
OpenTelemetryMiddleware since the non-standard fields do not exist on
the ASGI HTTP connection scope until after the subsequent middleware
runs and adds the expected fields.
@xrmx xrmx moved this to Ready for review in @xrmx's Python PR digest Aug 22, 2025
Copy link
Member

@emdneto emdneto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs CI fix and asserts in the test

@xrmx xrmx requested a review from emdneto October 10, 2025 09:39
@xrmx xrmx requested a review from a team as a code owner October 14, 2025 15:20
@xrmx xrmx moved this from Ready for review to Approved PRs in @xrmx's Python PR digest Oct 14, 2025
@xrmx xrmx merged commit ba0644f into open-telemetry:main Oct 14, 2025
638 checks passed
@github-project-automation github-project-automation bot moved this from Approved PRs to Done in @xrmx's Python PR digest Oct 14, 2025
@lannuttia lannuttia deleted the fix/issue-3671 branch October 14, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

FastAPI Instrumentation Bypasses Middleware for Custom Router

3 participants